Conversation
c09fa4f to
00621cc
Compare
gerrod3
left a comment
There was a problem hiding this comment.
Looks good, mainly have clarifying questions before approval.
There was a problem hiding this comment.
Can you remind me again how these named changelog entries work? Why not file an issue for them?
There was a problem hiding this comment.
They will appear without as link.
|
|
||
|
|
||
| class PulpTestContext(PulpContext): | ||
| # TODO check if we can just make the base class ignore echo. |
There was a problem hiding this comment.
What does this mean, doesn't the base class raise a NotImplemented error?
There was a problem hiding this comment.
It does, and that means, the PulpContext is unusable as is. I think that is a bad thing.
|
|
||
| from pulp_glue.common.context import _REGISTERED_API_QUIRKS, PulpContext | ||
|
|
||
| pytestmark = pytest.mark.glue |
There was a problem hiding this comment.
Is this another pytest quirk? How does it work?
There was a problem hiding this comment.
It marks all tests in this scope. (Putting it in conftest did not work sadly.)
| @pytest.fixture(scope="session") | ||
| def pulp_cli_settings_path( |
| if hasattr(plugin, "mount"): | ||
| plugin.mount() |
There was a problem hiding this comment.
Currently none of the pulp_glue plugins have a mount method, do you see the need for one in the future?
There was a problem hiding this comment.
I'm not sure. This would be better "have" than "need".
| repository = "https://github.com/pulp/pulp-cli" | ||
| changelog = "https://docs.pulpproject.org/pulp_cli/CHANGES/" | ||
|
|
||
| [project.entry-points."pulp_glue.plugins"] |
There was a problem hiding this comment.
Have you opened up the corresponding PRs in our other cli-plugin repositories? They won't be registered as loaded until they have made this change.
There was a problem hiding this comment.
That is right, but i think it should be safe to fade this in slowly.
This allows to auto-detect and load all installed pulp-glue plugins. It is primarily useful for workflows where knowing all sub-types of an Entity is only important at runtime.
Review Checklist: